-
Notifications
You must be signed in to change notification settings - Fork 15.3k
Add FatLTO support for COFF #165529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add FatLTO support for COFF #165529
Conversation
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-clang-driver Author: Stefan Schmidt (thrimbor) ChangesThis adds support for FatLTO to COFF targets in clang and lld. The changes are adapted from 610fc5c and 14e3bec but much smaller because it just needed the COFF-specific parts wired in, and I tried my best to adapt the pre-existing ELF tests for the COFF version. My main goal is to be able to use this for shipping pre-built https://github.com/XboxDev/nxdk container images someday, which uses the Full diff: https://github.com/llvm/llvm-project/pull/165529.diff 10 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 79edc561c551f..28cc91c620b0c 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7789,7 +7789,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
options::OPT_fno_fat_lto_objects)) {
if (IsUsingLTO && A->getOption().matches(options::OPT_ffat_lto_objects)) {
assert(LTOMode == LTOK_Full || LTOMode == LTOK_Thin);
- if (!Triple.isOSBinFormatELF()) {
+ if (!Triple.isOSBinFormatELF() && !Triple.isOSBinFormatCOFF()) {
D.Diag(diag::err_drv_unsupported_opt_for_target)
<< A->getAsString(Args) << TC.getTripleString();
}
diff --git a/clang/test/CodeGen/fat-lto-objects.c b/clang/test/CodeGen/fat-lto-objects.c
index 36a73684e7bfe..64ccd8ae75a4d 100644
--- a/clang/test/CodeGen/fat-lto-objects.c
+++ b/clang/test/CodeGen/fat-lto-objects.c
@@ -35,6 +35,9 @@
// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=full -ffat-lto-objects -fsplit-lto-unit -S < %s -o - \
// RUN: | FileCheck %s --check-prefixes=ASM
+// RUN: %clang -cc1 -triple i386-pc-win32 -flto=full -ffat-lto-objects -fsplit-lto-unit -S < %s -o - \
+// RUN: | FileCheck %s --check-prefixes=ASM-COFF
+
/// Make sure that FatLTO generates .llvm.lto sections that are the same as the output from normal LTO compilations
// RUN: %clang -O2 --target=x86_64-unknown-linux-gnu -fPIE -flto=full -ffat-lto-objects -c %s -o %t.fatlto.full.o
// RUN: llvm-objcopy --dump-section=.llvm.lto=%t.fatlto.full.bc %t.fatlto.full.o
@@ -43,6 +46,13 @@
// RUN: llvm-dis < %t.nofat.full.bc -o %t.nofat.full.ll
// RUN: diff %t.fatlto.full.ll %t.nofat.full.ll
+// RUN: %clang -O2 --target=i386-pc-win32 -flto=full -ffat-lto-objects -c %s -o %t.fatlto.full.coff.o
+// RUN: llvm-objcopy --dump-section=.llvm.lto=%t.fatlto.full.coff.bc %t.fatlto.full.coff.o
+// RUN: llvm-dis < %t.fatlto.full.coff.bc -o %t.fatlto.full.coff.ll
+// RUN: %clang -O2 --target=i386-pc-win32 -flto=full -c %s -o %t.nofat.full.coff.bc
+// RUN: llvm-dis < %t.nofat.full.coff.bc -o %t.nofat.full.coff.ll
+// RUN: diff %t.fatlto.full.coff.ll %t.nofat.full.coff.ll
+
// RUN: %clang -O2 --target=x86_64-unknown-linux-gnu -fPIE -flto=thin -ffat-lto-objects -c %s -o %t.fatlto.thin.o
// RUN: llvm-objcopy --dump-section=.llvm.lto=%t.fatlto.thin.bc %t.fatlto.thin.o
// RUN: llvm-dis < %t.fatlto.thin.bc -o %t.fatlto.thin.ll
@@ -67,6 +77,10 @@
// ASM-NEXT: .asciz "BC
// ASM-NEXT: .size .Lllvm.embedded.object
+// ASM-COFF: .section .llvm.lto,"ynD"
+// ASM-COFF-NEXT: L_llvm.embedded.object:
+// ASM-COFF-NEXT: .asciz "BC
+
const char* foo = "foo";
int test(void) {
diff --git a/clang/test/Driver/fat-lto-objects.c b/clang/test/Driver/fat-lto-objects.c
index 7b87e2b468886..84dd945e208b9 100644
--- a/clang/test/Driver/fat-lto-objects.c
+++ b/clang/test/Driver/fat-lto-objects.c
@@ -1,10 +1,12 @@
// RUN: %clang --target=x86_64-unknown-linux-gnu -flto -ffat-lto-objects -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC
+// RUN: %clang --target=i386-pc-win32 -flto -ffat-lto-objects -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC
// CHECK-CC: -cc1
// CHECK-CC-SAME: -emit-obj
// CHECK-CC-SAME: -ffat-lto-objects
/// Without -flto -S will just emit normal ASM, so we don't expect -emit-{llvm,obj} or -ffat-lto-objects to be passed to cc1.
// RUN: %clang --target=x86_64-unknown-linux-gnu -ffat-lto-objects -### %s -S 2>&1 | FileCheck %s -check-prefix=CHECK-CC-S
+// RUN: %clang --target=i386-pc-win32 -ffat-lto-objects -### %s -S 2>&1 | FileCheck %s -check-prefix=CHECK-CC-S
// CHECK-CC-S: -cc1
// CHECK-CC-S: -S
// CHECK-CC-S-NOT: -emit-obj
@@ -13,35 +15,41 @@
/// When fat LTO is enabled with -S, we expect asm output and -ffat-lto-objects to be passed to cc1.
// RUN: %clang --target=x86_64-unknown-linux-gnu -flto -ffat-lto-objects -### %s -S 2>&1 | FileCheck %s -check-prefix=CHECK-CC-S-LTO
+// RUN: %clang --target=i386-pc-win32 -flto -ffat-lto-objects -### %s -S 2>&1 | FileCheck %s -check-prefix=CHECK-CC-S-LTO
// CHECK-CC-S-LTO: -cc1
// CHECK-CC-S-NOT: -emit-llvm
// CHECK-CC-S-LTO-SAME: -ffat-lto-objects
/// When fat LTO is enabled with -S and -emit-llvm, we expect IR output and -ffat-lto-objects to be passed to cc1.
// RUN: %clang --target=x86_64-unknown-linux-gnu -flto -ffat-lto-objects -### %s -S -emit-llvm 2>&1 | FileCheck %s -check-prefix=CHECK-CC-S-EL-LTO
+// RUN: %clang --target=i386-pc-win32 -flto -ffat-lto-objects -### %s -S -emit-llvm 2>&1 | FileCheck %s -check-prefix=CHECK-CC-S-EL-LTO
// CHECK-CC-S-EL-LTO: -cc1
// CHECK-CC-S-EL-LTO-SAME: -emit-llvm
// CHECK-CC-S-EL-LTO-SAME: -ffat-lto-objects
/// When fat LTO is enabled without -S we expect native object output and -ffat-lto-object to be passed to cc1.
// RUN: %clang --target=x86_64-unknown-linux-gnu -flto -ffat-lto-objects -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-C-LTO
+// RUN: %clang --target=i386-pc-win32 -flto -ffat-lto-objects -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-C-LTO
// CHECK-CC-C-LTO: -cc1
// CHECK-CC-C-LTO-SAME: -emit-obj
// CHECK-CC-C-LTO-SAME: -ffat-lto-objects
/// When fat LTO is enabled with -c and -emit-llvm we expect bitcode output and -ffat-lto-object to be passed to cc1.
// RUN: %clang --target=x86_64-unknown-linux-gnu -flto -ffat-lto-objects -### %s -c -emit-llvm 2>&1 | FileCheck %s -check-prefix=CHECK-CC-C-EL-LTO
+// RUN: %clang --target=i386-pc-win32 -flto -ffat-lto-objects -### %s -c -emit-llvm 2>&1 | FileCheck %s -check-prefix=CHECK-CC-C-EL-LTO
// CHECK-CC-C-EL-LTO: -cc1
// CHECK-CC-C-EL-LTO-SAME: -emit-llvm-bc
// CHECK-CC-C-EL-LTO-SAME: -ffat-lto-objects
/// Make sure we don't have a warning for -ffat-lto-objects being unused
// RUN: %clang --target=x86_64-unknown-linux-gnu -ffat-lto-objects -fdriver-only -Werror -v %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-NOLTO
+// RUN: %clang --target=i386-pc-win32 -ffat-lto-objects -fdriver-only -Werror -v %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-NOLTO
// CHECK-CC-NOLTO: -cc1
// CHECK-CC-NOLTO-SAME: -emit-obj
// CHECK-CC-NOLTO-NOT: -ffat-lto-objects
// RUN: %clang --target=x86_64-unknown-linux-gnu -flto -ffat-lto-objects -fno-fat-lto-objects -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-NOLTO
+// RUN: %clang --target=i386-pc-win32 -flto -ffat-lto-objects -fno-fat-lto-objects -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-NOLTO
/// We need to pass an additional flag (--fat-lto-objects) to lld when linking w/ -flto -ffat-lto-objects
/// But it should not be there when LTO is disabled w/ -fno-lto
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 16997e4de780e..c5c0338c99e57 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -207,6 +207,9 @@ struct Configuration {
// Used for /thinlto-remote-compiler-arg:<arg>
llvm::SmallVector<llvm::StringRef, 0> dtltoCompilerArgs;
+ // Used for /fat-lto-objects
+ bool fatLTOObjects = false;
+
// Used for /opt:[no]ltodebugpassmanager
bool ltoDebugPassManager = false;
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 0e528de9c3652..ac5942a3ac2d8 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -28,6 +28,7 @@
#include "llvm/Config/llvm-config.h"
#include "llvm/LTO/LTO.h"
#include "llvm/Object/COFFImportFile.h"
+#include "llvm/Object/IRObjectFile.h"
#include "llvm/Option/Arg.h"
#include "llvm/Option/ArgList.h"
#include "llvm/Option/Option.h"
@@ -256,6 +257,23 @@ MemoryBufferRef LinkerDriver::takeBuffer(std::unique_ptr<MemoryBuffer> mb) {
return mbref;
}
+InputFile *LinkerDriver::tryCreateFatLTOFile(MemoryBufferRef mb,
+ StringRef archiveName,
+ uint64_t offsetInArchive,
+ bool lazy) {
+ if (!ctx.config.fatLTOObjects)
+ return nullptr;
+
+ Expected<MemoryBufferRef> fatLTOData =
+ IRObjectFile::findBitcodeInMemBuffer(mb);
+
+ if (errorToBool(fatLTOData.takeError()))
+ return nullptr;
+
+ return BitcodeFile::create(ctx, *fatLTOData, archiveName, offsetInArchive,
+ lazy);
+}
+
void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> mb,
bool wholeArchive, bool lazy) {
StringRef filename = mb->getBufferIdentifier();
@@ -289,7 +307,14 @@ void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> mb,
case file_magic::bitcode:
addFile(BitcodeFile::create(ctx, mbref, "", 0, lazy));
break;
- case file_magic::coff_object:
+ case file_magic::coff_object: {
+ InputFile *obj = tryCreateFatLTOFile(mbref, "", 0, lazy);
+ if (obj)
+ addFile(obj);
+ else
+ addFile(ObjFile::create(ctx, mbref, lazy));
+ break;
+ }
case file_magic::coff_import_library:
addFile(ObjFile::create(ctx, mbref, lazy));
break;
@@ -374,7 +399,9 @@ void LinkerDriver::addArchiveBuffer(MemoryBufferRef mb, StringRef symName,
InputFile *obj;
if (magic == file_magic::coff_object) {
- obj = ObjFile::create(ctx, mb);
+ obj = tryCreateFatLTOFile(mb, parentName, offsetInArchive, /*lazy=*/false);
+ if (!obj)
+ obj = ObjFile::create(ctx, mb);
} else if (magic == file_magic::bitcode) {
obj = BitcodeFile::create(ctx, mb, parentName, offsetInArchive,
/*lazy=*/false);
@@ -2121,6 +2148,10 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
config->dtltoCompilerArgs =
args::getStrings(args, OPT_thinlto_remote_compiler_arg);
+ // Handle /fat-lto-objects
+ config->fatLTOObjects =
+ args.hasFlag(OPT_fat_lto_objects, OPT_fat_lto_objects_no, false);
+
// Handle /dwodir
config->dwoDir = args.getLastArgValue(OPT_dwodir);
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index 14710d5853bcf..8b28bd9f3c617 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -169,6 +169,8 @@ class LinkerDriver {
std::set<std::string> visitedLibs;
+ InputFile *tryCreateFatLTOFile(MemoryBufferRef mb, StringRef archiveName,
+ uint64_t offsetInArchive, bool lazy);
void addBuffer(std::unique_ptr<MemoryBuffer> mb, bool wholeArchive,
bool lazy);
void addArchiveBuffer(MemoryBufferRef mbref, StringRef symName,
diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
index d77478fc9c987..911c5c55f249b 100644
--- a/lld/COFF/Options.td
+++ b/lld/COFF/Options.td
@@ -298,6 +298,9 @@ def thinlto_remote_compiler_prepend_arg : P<"thinlto-remote-compiler-prepend-arg
def thinlto_remote_compiler_arg : P<"thinlto-remote-compiler-arg",
"Compiler arguments for the ThinLTO distributor to pass for ThinLTO backend "
"compilations">;
+defm fat_lto_objects: B<"fat-lto-objects",
+ "Use the .llvm.lto section, which contains LLVM bitcode, in fat LTO object files to perform LTO.",
+ "Ignore the .llvm.lto section in relocatable object files (default).">;
def lto_obj_path : P<
"lto-obj-path",
"output native object for merged LTO unit to this path">;
diff --git a/lld/test/COFF/fatlto/fatlto-invalid.s b/lld/test/COFF/fatlto/fatlto-invalid.s
new file mode 100644
index 0000000000000..5e60633ef5f11
--- /dev/null
+++ b/lld/test/COFF/fatlto/fatlto-invalid.s
@@ -0,0 +1,8 @@
+# RUN: llvm-mc -filetype=obj -triple=i386-pc-win32 %s -o %t
+# RUN: env LLD_IN_TEST=1 not lld-link %t /out:/dev/null /fat-lto-objects 2>&1 | FileCheck %s
+
+# CHECK: error:{{.*}} Invalid bitcode signature
+
+.section .llvm.lto,"ynD"
+L_llvm.embedded.object:
+ .asciz "BC\300\3365\000"
diff --git a/lld/test/COFF/fatlto/fatlto.test b/lld/test/COFF/fatlto/fatlto.test
new file mode 100644
index 0000000000000..19b62a94ea21a
--- /dev/null
+++ b/lld/test/COFF/fatlto/fatlto.test
@@ -0,0 +1,111 @@
+;; Basic FatLTO tests.
+; REQUIRES: x86
+
+; RUN: rm -rf %t && split-file %s %t && cd %t
+
+;; Ensure that input files contain .llvm.lto section.
+; RUN: llc a-LTO.ll --filetype=obj -o a-fatLTO.o --relocation-model=pic
+; RUN: opt < a-LTO.ll --module-summary -o a-fatLTO.bc
+; RUN: llvm-objcopy --add-section=.llvm.lto=a-fatLTO.bc --set-section-flags=.llvm.lto=exclude a-fatLTO.o
+
+; RUN: llc main-LTO.ll --filetype=obj -o main-fatLTO.o --relocation-model=pic
+; RUN: opt < main-LTO.ll --module-summary -o main-fatLTO.bc
+; RUN: llvm-objcopy --add-section=.llvm.lto=main-fatLTO.bc --set-section-flags=.llvm.lto=exclude main-fatLTO.o
+
+; RUN: llvm-readobj --sections a-fatLTO.o | FileCheck --check-prefix=HAS_LLVM_LTO %s
+; RUN: llvm-readobj --sections main-fatLTO.o | FileCheck --check-prefix=HAS_LLVM_LTO %s
+
+;; Make sure that the section flags are set correctly
+; HAS_LLVM_LTO: Name: .llvm.lto (2F 34 00 00 00 00 00 00)
+; HAS_LLVM_LTO: Characteristics [
+; HAS_LLVM_LTO-SAME: (0xC0000800)
+; HAS_LLVM_LTO-NEXT: IMAGE_SCN_LNK_REMOVE (0x800)
+; HAS_LLVM_LTO-NEXT: IMAGE_SCN_MEM_READ (0x40000000)
+; HAS_LLVM_LTO-NEXT: IMAGE_SCN_MEM_WRITE (0x80000000)
+; HAS_LLVM_LTO-NEXT: ]
+
+;; Final executable should not have .llvm.lto section no matter what the target is.
+; RUN: lld-link /timestamp:0 /out:foo-fatLTO /entry:main a-fatLTO.o main-fatLTO.o -fat-lto-objects
+; RUN: llvm-readobj --sections foo-fatLTO | FileCheck --check-prefix=CHECK-LTO-TARGET %s
+
+;; Check that fat objects work w/ --start-lib.
+; RUN: lld-link /out:foo-fatLTO.start_lib /entry:main -start-lib a-fatLTO.o main-fatLTO.o -fat-lto-objects
+; RUN: llvm-readobj --sections foo-fatLTO.start_lib | FileCheck --check-prefix=CHECK-LTO-TARGET %s
+
+;; Check if .llvm.lto section gets aggregated in LTO target.
+; CHECK-LTO-TARGET-NOT: Name: .llvm.lto
+
+;; Final executable should not have .llvm.lto section no matter what the target is.
+; RUN: lld-link /timestamp:0 /out:foo-fatNoLTO /entry:main a-fatLTO.o %/t/main-fatLTO.o
+; RUN: llvm-readobj --sections foo-fatNoLTO | FileCheck --check-prefix=CHECK-NON-LTO-TARGET %s
+
+;; Check if .llvm.lto section gets aggregated in non-LTO target.
+; CHECK-NON-LTO-TARGET-NOT: Name: .llvm.lto
+
+;; Check if the LTO target executable produced from FatLTO object file is
+;; identical to the one produced from LTO modules.
+; RUN: opt < a-LTO.ll --module-summary -o a-LTO.bc
+; RUN: opt < main-LTO.ll --module-summary -o main-LTO.bc
+; RUN: lld-link /timestamp:0 /out:foo-LTO /entry:main a-LTO.bc main-LTO.bc
+; RUN: cmp foo-fatLTO foo-LTO
+
+;; Check if the no-LTO target executable produced from FatLTO object file is
+;; identical to the one produced from regular object files.
+; RUN: llc a-LTO.ll --filetype=obj -o a.o
+; RUN: llc main-LTO.ll --filetype=obj -o main.o
+; RUN: lld-link /timestamp:0 /out:foo-noLTO /entry:main a.o main.o
+; RUN: cmp foo-fatNoLTO foo-noLTO
+
+;; Check archive support.
+; RUN: llvm-ar rcs a.a a-fatLTO.o
+; RUN: lld-link /timestamp:0 /out:foo-fatLTO.archive /entry:main /wholearchive a.a main-LTO.bc -fat-lto-objects
+; RUN: cmp foo-fatLTO.archive foo-LTO
+
+;--- a-LTO.ll
+target datalayout = "e-m:x-p:32:32-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32-a:0:32-S32"
+target triple = "i386-pc-windows-msvc19.33.0"
+
+; Function Attrs: noinline nounwind uwtable
+define dso_local i32 @_start() #0 {
+entry:
+ ret i32 0
+}
+
+attributes #0 = { noinline nounwind uwtable }
+
+!llvm.module.flags = !{!0, !1, !2, !3, !4, !5, !6}
+
+!0 = !{i32 1, !"NumRegisterParameters", i32 0}
+!1 = !{i32 2, !"Debug Info Version", i32 3}
+!2 = !{i32 1, !"wchar_size", i32 2}
+!3 = !{i32 7, !"frame-pointer", i32 2}
+!4 = !{i32 1, !"MaxTLSAlign", i32 65536}
+!5 = !{i32 1, !"ThinLTO", i32 0}
+!6 = !{i32 1, !"EnableSplitLTOUnit", i32 1}
+
+;--- main-LTO.ll
+target datalayout = "e-m:x-p:32:32-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32-a:0:32-S32"
+target triple = "i386-pc-windows-msvc19.33.0"
+
+; Function Attrs: noinline nounwind uwtable
+define dso_local i32 @main() #0 {
+entry:
+ %retval = alloca i32, align 4
+ store i32 0, ptr %retval, align 4
+ %call = call i32 (...) @_start()
+ ret i32 %call
+}
+
+declare i32 @_start(...)
+
+attributes #0 = { noinline nounwind uwtable }
+
+!llvm.module.flags = !{!0, !1, !2, !3, !4, !5, !6}
+
+!0 = !{i32 1, !"NumRegisterParameters", i32 0}
+!1 = !{i32 2, !"Debug Info Version", i32 3}
+!2 = !{i32 1, !"wchar_size", i32 2}
+!3 = !{i32 7, !"frame-pointer", i32 2}
+!4 = !{i32 1, !"MaxTLSAlign", i32 65536}
+!5 = !{i32 1, !"ThinLTO", i32 0}
+!6 = !{i32 1, !"EnableSplitLTOUnit", i32 1}
diff --git a/llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp b/llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp
index f02256f8a83b9..1c9183c03dab4 100644
--- a/llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp
+++ b/llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp
@@ -27,7 +27,7 @@ PreservedAnalyses EmbedBitcodePass::run(Module &M, ModuleAnalysisManager &AM) {
reportFatalUsageError("Can only embed the module once");
Triple T(M.getTargetTriple());
- if (T.getObjectFormat() != Triple::ELF)
+ if (T.getObjectFormat() != Triple::ELF && T.getObjectFormat() != Triple::COFF)
reportFatalUsageError(
"EmbedBitcode pass currently only supports ELF object format");
|
tru
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this. It needs a release note before it can be accepted.
| if (T.getObjectFormat() != Triple::ELF) | ||
| if (T.getObjectFormat() != Triple::ELF && T.getObjectFormat() != Triple::COFF) | ||
| reportFatalUsageError( | ||
| "EmbedBitcode pass currently only supports ELF object format"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This string needs to be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
234945b to
e40ae66
Compare
|
I rebased the PR and added a ReleaseNote entry for LLD based on the one that was added for ELF FatLTO. This is the first time I've done that, so if there's something not quite right with it please let me know. |
tru
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now, but wait for at least one more reviewer that looks at the code.
|
Friendly ping for review/merge @mstorsjo @nikic @bd1976bris @aganea @MaskRay |
| // ASM-NEXT: .asciz "BC | ||
| // ASM-NEXT: .size .Lllvm.embedded.object | ||
|
|
||
| // ASM-COFF: .section .llvm.lto,"ynD" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align the prefix on the left side. You can add whitespaces to align the pattern if desired, they will be ignored by FileCheck.
| return mbref; | ||
| } | ||
|
|
||
| InputFile *LinkerDriver::tryCreateFatLTOFile(MemoryBufferRef mb, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this as a member function, it could just be static.
Additionnaly, can we also handle the ObjFile::create call in this function please?
| Expected<MemoryBufferRef> fatLTOData = | ||
| IRObjectFile::findBitcodeInMemBuffer(mb); | ||
|
|
||
| if (errorToBool(fatLTOData.takeError())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not segregate between "we haven't found the bitstream in the COFF object" vs. "there's an error while parsing the object". If the return code is object_error::bitcode_section_not_found we should fallback on the ObjFile::create case; whereas any other error should be emitted in the log (Fatal(ctx) << ...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is true that any COFF error will be handled later by ObjFile::create. I am wondering if IRObjectFile::findBitcodeInMemBuffer could return other errors while looking for the FAT LTO buffer (not just while parsing the COFF headers)
| @@ -0,0 +1,8 @@ | |||
| # RUN: llvm-mc -filetype=obj -triple=i386-pc-win32 %s -o %t | |||
| # RUN: env LLD_IN_TEST=1 not lld-link %t /out:/dev/null /fat-lto-objects 2>&1 | FileCheck %s | |||
|
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you able to handle a COFF error here as well? For example, when dealing with a FAT LTO .OBJ, any of the object_error::parse_failed returned by COFFObjectFile::initialize?
|
I had a quick readthrough, and didn't find anything objectionable on top of what @aganea already pointed out. The PR seems to have two separate commits though, that I feel would benefit of staying split into two separate commits in the end. I.e. that would require splitting out at least the Clang change to a separate PR after the LLD change. (The change to EmbedBitcodePass probably can go along with the LLD change, I guess it's required for that to work.) Then as a small request, I'd appreciate if you could hook up this new option to the LLD MinGW frontend as well. That just requires adding a similar option there (usually it's possible to copy over the option definition from the ELF linker) which just forwards this option to the internal lld-link interface. I guess that'd also be a separate PR aside from this. |
This adds support for FatLTO to COFF targets in clang and lld.
The changes are adapted from 610fc5c and 14e3bec but much smaller because it just needed the COFF-specific parts wired in, and I tried my best to adapt the pre-existing ELF tests for the COFF version.
My main goal is to be able to use this for shipping pre-built https://github.com/XboxDev/nxdk container images someday, which uses the
i386-pc-win32target.